Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cosmos: Redo ReadItem and partition key management #34187

Merged
merged 4 commits into from
Jul 9, 2024

Conversation

roji
Copy link
Member

@roji roji commented Jul 8, 2024

  • Rewrote partition key & read item detection to operate as the final postprocessing step, pattern-matching on the final SQL tree, rather than on incoming pre-translated LINQ expressions. This removes the need to pattern-match each and every construct (== and Equals, EF.Property, indexer properties...), and simply performs the transformation whenever the final SQL form is compatible.
  • Implemented ReadItem transformation rules for hierarchy vs. shared containers; we no longer transform to ReadItem for base types in a hierarchy.
  • Unified the partition key management & flow schemes (we had no less than three). Now regardless of how the partition key is found/managed (WithPartitionKey(), predicate comparison extraction, ReadItem), the values are simply set on CosmosQueryCompialtionContext.PartitionKey.
  • Improved WithPartitionKey() for better support for parameterizing partition key properties: previously [NonParameterized] was used, causing us to recompile queries with different WithPartitionKey arguments. We now properly parameterize and use a single compilation.
  • Move WithPartitionKey() processing from preprocessing to translation (so that we can get Sql{Constant,Parameter}Expression)
  • Brought across SqlExpressionFactory optimizations that have recently been done in relational

Closes #20693
Closes #20350
Fixes #34085
Fixes #33960

@roji roji requested a review from a team July 8, 2024 14:35
@roji roji changed the title Redo ReadItem and partition key management Cosmos: Redo ReadItem and partition key management Jul 8, 2024
@roji roji force-pushed the CosmosReadItem branch 2 times, most recently from ff6c096 to 499e04f Compare July 8, 2024 18:28
@@ -56,8 +54,8 @@ public static class CosmosQueryableExtensions
/// <returns>A new query with the set partition key.</returns>
public static IQueryable<TEntity> WithPartitionKey<TEntity>(
this IQueryable<TEntity> source,
[NotParameterized] object partitionKeyValue,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This caused us to recompile the query for each different constant value of a partition key passed to WithPartitionKey()... We now support proper parameterization.

@roji roji requested a review from AndriySvyryd July 9, 2024 15:20
@roji roji enabled auto-merge (squash) July 9, 2024 18:38
@roji roji merged commit 041f4f2 into dotnet:main Jul 9, 2024
7 checks passed
@roji roji deleted the CosmosReadItem branch July 9, 2024 19:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants